refactor #116 remove callback_type #154

BrettRD · 2021-07-21T04:07:54Z

This refactor eliminates handle->callback_type, and expands handle->type to all available rclc_executor_add_*() functions.
This resolves both: #116 and #115

I've permitted NULL contexts for services, so that we can eventually drop the API that relies on global variables.

rclc_executor_remove_* can be drastically simplified if the check for handle->type is dropped, moving the search into a helper function and leveraging rclc_executor_handle_get_ptr

@jst3si left suggestions for a new service message and response struct, which look like a good idea for rolling.

Signed-off-by: BrettRD <[email protected]>

codecov-commenter · 2021-07-21T04:14:08Z

Codecov Report

Merging #154 (0c3c2db) into foxy (1ac4a28) will increase coverage by 0.72%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             foxy     #154      +/-   ##
==========================================
+ Coverage   28.06%   28.78%   +0.72%     
==========================================
  Files          28       28              
  Lines        3403     3380      -23     
  Branches     1804     1785      -19     
==========================================
+ Hits          955      973      +18     
+ Misses        912      892      -20     
+ Partials     1536     1515      -21

Impacted Files	Coverage Δ
...os_ws/src/td72ocxi5ao/rclc/rclc/src/rclc/service.c
...rclc_lifecycle/src/rclc_lifecycle/rclc_lifecycle.c
...src/td72ocxi5ao/rclc/rclc/test/rclc/test_timer.cpp
...i5ao/rclc/rclc_examples/src/example_service_node.c
...2ocxi5ao/rclc/rclc/test/rclc/test_subscription.cpp
ros_ws/src/td72ocxi5ao/rclc/rclc/src/rclc/client.c
.../rclc/rclc_examples/src/example_executor_trigger.c
...xi5ao/rclc/rclc_examples/src/example_sub_context.c
ros_ws/src/td72ocxi5ao/rclc/rclc/src/rclc/sleep.c
.../src/td72ocxi5ao/rclc/rclc/src/rclc/subscription.c
... and 46 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ac4a28...0c3c2db. Read the comment docs.

Signed-off-by: BrettRD <[email protected]>

JanStaschulat · 2021-07-28T09:53:38Z

rclc/src/rclc/executor.c

-        return ret;
-      }
-    }
+  rclc_executor_handle_t * handle = _rclc_executor_find_handle(executor, subscription);


You considered it not necessary to check for the type? Of course, you could argue that if the pointer matches then it must be the same object (with the same type). LGTM.

JanStaschulat · 2021-07-28T10:01:46Z

rclc/src/rclc/executor.c

+          return false;
+        }
+        if (obj == handle_obj_ptr) {
+          return true;


JanStaschulat · 2021-07-28T10:06:35Z

@mergify backport galactic rolling

* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24)

mergify · 2021-07-28T10:07:00Z

Command backport galactic rolling: failure

No backport have been created

#163 refactor #116 remove callback_type (backport #154) has been created for branch galactic

Backport to branch rolling failed: Branch not found

Hey, I reacted but my real name is @Mergifyio

JanStaschulat · 2021-07-28T10:13:42Z

@mergify backport master

* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24)

mergify · 2021-07-28T10:14:00Z

Command backport master: success

Backports have been created

#164 refactor #116 remove callback_type (backport #154) has been created for branch master

Hey, I reacted but my real name is @Mergifyio

* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]>

* refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]>

* ci workflow for galactic (#90) Signed-off-by: Jan Staschulat <[email protected]> * galactic: updated ci-job and codecov (#91) * bump version to 2.0.1 of galactic branch for new bloom release (#95) * Dummy (#100) * test of galactic bloom release see also #100 (#101) * Adds a context pointer to subscriptions (backport #107) (#120) * Adds a context pointer to subscriptions (#107) (cherry picked from commit 5b11c5a) # Conflicts: # rclc_examples/CMakeLists.txt * resolved merge confict Signed-off-by: Jan Staschulat <[email protected]> Co-authored-by: Brett Downing <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> * version bump and updated changelog. (#139) Signed-off-by: Jan Staschulat <[email protected]> * prepare bloom release v2.0.3 for galactic (#159) Signed-off-by: Jan Staschulat <[email protected]> * refactor #116 remove callback_type (#154) (#163) * refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]> * updated rcl dependency to galactic (#184) Signed-off-by: Jan Staschulat <[email protected]> * added pingpong example (#172) (#188) * added pingpong example Signed-off-by: Jan Staschulat <[email protected]> * added two executor configuration to pingpong Signed-off-by: Jan Staschulat <[email protected]> * example_pingpong multiple files Signed-off-by: Jan Staschulat <[email protected]> * added class member function to executor Signed-off-by: Jan Staschulat <[email protected]> (cherry picked from commit e0252bd) Co-authored-by: Jan Staschulat <[email protected]> * prepare bloom release 2.0.4-galactic (#193) Signed-off-by: Jan Staschulat <[email protected]> * Copy client to action client Initial mods to action client Initial executor support for action client Updates Fix Updates Copy client action to server action Initial action server changes Fix Revert "Copy client to action client" This reverts commit 304c70a. Initial Update Working state Uncrustify fix Update Update Add client Uncrustify Uncrus Apply suggestions from code review Co-authored-by: Ralph Lange <[email protected]> Initial test Fix data available logic Update tests Update Uncrust Revert "Update" This reverts commit 940aa96. Working state Working Building state Update Working state Working state Working state Uncrust Updates Fix cancelling reject Test approach modified Complete server tests Uncrusti Add multigoal test Fix Initial action client tests Add executor client cancel handlers Update action client Add cancel tests Tests update Uncrustify Refactor cancel response handling Update * Revert "Copy client to action client" This reverts commit d65adc7. * Initial version * Hide non public API * Fix abort API * Minor fix * Update * Fix * Add rclcpp dep * Update actions implementation * Delete COLCON_IGNORE * Fix dependencies export * cpplint * Initial Ralph changes Co-authored-by: Ralph Lange <[email protected]> * Add RCLC error * Refactor pop/take inner functions * Argument renaming * Argument renaming * Rename get API to find * Reafctor functio naming * Refactor naming * Remove explicit == true in conditionals * Update rclc/src/rclc/executor.c Co-authored-by: Ralph Lange <[email protected]> * Update rclc/src/rclc/executor.c Co-authored-by: Ralph Lange <[email protected]> * Remove white lines * Update rclc/src/rclc/action_server.c * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> * Add pointer check suggestions * Apply suggestions on executor * Add check for feedback msg on client * Put goal on default switch case * Revert code delete * Add comments to executor loops * Update loop comments * Apply for loop style where possible * Fix variable name * Fix rebase * ci workflow for galactic (#90) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * galactic: updated ci-job and codecov (#91) Signed-off-by: Antonio Cuadros <[email protected]> * bump version to 2.0.1 of galactic branch for new bloom release (#95) Signed-off-by: Antonio Cuadros <[email protected]> * Dummy (#100) Signed-off-by: Antonio Cuadros <[email protected]> * test of galactic bloom release see also #100 (#101) Signed-off-by: Antonio Cuadros <[email protected]> * Adds a context pointer to subscriptions (backport #107) (#120) * Adds a context pointer to subscriptions (#107) (cherry picked from commit 5b11c5a) # Conflicts: # rclc_examples/CMakeLists.txt * resolved merge confict Signed-off-by: Jan Staschulat <[email protected]> Co-authored-by: Brett Downing <[email protected]> Co-authored-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * version bump and updated changelog. (#139) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * prepare bloom release v2.0.3 for galactic (#159) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * refactor #116 remove callback_type (#154) (#163) * refactor #116 remove callback_type Signed-off-by: BrettRD <[email protected]> * rename subscription callback Signed-off-by: BrettRD <[email protected]> * deduplicate remove functions Signed-off-by: BrettRD <[email protected]> (cherry picked from commit 700ea24) Co-authored-by: Brett Downing <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * updated rcl dependency to galactic (#184) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * added pingpong example (#172) (#188) * added pingpong example Signed-off-by: Jan Staschulat <[email protected]> * added two executor configuration to pingpong Signed-off-by: Jan Staschulat <[email protected]> * example_pingpong multiple files Signed-off-by: Jan Staschulat <[email protected]> * added class member function to executor Signed-off-by: Jan Staschulat <[email protected]> (cherry picked from commit e0252bd) Co-authored-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * prepare bloom release 2.0.4-galactic (#193) Signed-off-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Copy client to action client Initial mods to action client Initial executor support for action client Updates Fix Updates Copy client action to server action Initial action server changes Fix Revert "Copy client to action client" This reverts commit 304c70a. Initial Update Working state Uncrustify fix Update Update Add client Uncrustify Uncrus Apply suggestions from code review Co-authored-by: Ralph Lange <[email protected]> Initial test Fix data available logic Update tests Update Uncrust Revert "Update" This reverts commit 940aa96. Working state Working Building state Update Working state Working state Working state Uncrust Updates Fix cancelling reject Test approach modified Complete server tests Uncrusti Add multigoal test Fix Initial action client tests Add executor client cancel handlers Update action client Add cancel tests Tests update Uncrustify Refactor cancel response handling Update Signed-off-by: Antonio Cuadros <[email protected]> * Revert "Copy client to action client" This reverts commit d65adc7. Signed-off-by: Antonio Cuadros <[email protected]> * Initial version Signed-off-by: Antonio Cuadros <[email protected]> * Hide non public API Signed-off-by: Antonio Cuadros <[email protected]> * Fix abort API Signed-off-by: Antonio Cuadros <[email protected]> * Minor fix Signed-off-by: Antonio Cuadros <[email protected]> * Update Signed-off-by: Antonio Cuadros <[email protected]> * Fix Signed-off-by: Antonio Cuadros <[email protected]> * Add rclcpp dep Signed-off-by: Antonio Cuadros <[email protected]> * Update actions implementation Signed-off-by: Antonio Cuadros <[email protected]> * Delete COLCON_IGNORE Signed-off-by: Antonio Cuadros <[email protected]> * Fix dependencies export Signed-off-by: Antonio Cuadros <[email protected]> * cpplint Signed-off-by: Antonio Cuadros <[email protected]> * Initial Ralph changes Co-authored-by: Ralph Lange <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Add RCLC error Signed-off-by: Antonio Cuadros <[email protected]> * Refactor pop/take inner functions Signed-off-by: Antonio Cuadros <[email protected]> * Argument renaming Signed-off-by: Antonio Cuadros <[email protected]> * Argument renaming Signed-off-by: Antonio Cuadros <[email protected]> * Rename get API to find Signed-off-by: Antonio Cuadros <[email protected]> * Reafctor functio naming Signed-off-by: Antonio Cuadros <[email protected]> * Refactor naming Signed-off-by: Antonio Cuadros <[email protected]> * Remove explicit == true in conditionals Signed-off-by: Antonio Cuadros <[email protected]> * Update rclc/src/rclc/executor.c Co-authored-by: Ralph Lange <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Update rclc/src/rclc/executor.c Co-authored-by: Ralph Lange <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Remove white lines Signed-off-by: Antonio Cuadros <[email protected]> * Update rclc/src/rclc/action_server.c Signed-off-by: Antonio Cuadros <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> Signed-off-by: Antonio Cuadros <[email protected]> * Add pointer check suggestions Signed-off-by: Antonio Cuadros <[email protected]> * Apply suggestions on executor Signed-off-by: Antonio Cuadros <[email protected]> * Add check for feedback msg on client Signed-off-by: Antonio Cuadros <[email protected]> * Put goal on default switch case Signed-off-by: Antonio Cuadros <[email protected]> * Revert code delete Signed-off-by: Antonio Cuadros <[email protected]> * Add comments to executor loops Signed-off-by: Antonio Cuadros <[email protected]> * Update loop comments Signed-off-by: Antonio Cuadros <[email protected]> * Apply for loop style where possible Signed-off-by: Antonio Cuadros <[email protected]> * Fix variable name Signed-off-by: Antonio Cuadros <[email protected]> * Fix rebase Signed-off-by: Antonio Cuadros <[email protected]> * Apply suggestions from code review Co-authored-by: Jan Staschulat <[email protected]> * removed unneccary reset of handle->data_available , in _rclc_check_for_new_data this flag is assigned again. Signed-off-by: Jan Staschulat (CR/ADA1.2) <[email protected]> * Change put goal function name Co-authored-by: Jan Staschulat <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Brett Downing <[email protected]> Co-authored-by: Antonio Cuadros <[email protected]> Co-authored-by: Ralph Lange <[email protected]> Co-authored-by: Antonio Cuadros <[email protected]>

refactor ros2#116 remove callback_type

6dafd9f

Signed-off-by: BrettRD <[email protected]>

BrettRD force-pushed the refactor_116 branch from c7d1175 to 6dafd9f Compare July 21, 2021 04:08

rename subscription callback

ab33e95

Signed-off-by: BrettRD <[email protected]>

BrettRD force-pushed the refactor_116 branch from 2bee5fd to ab33e95 Compare July 21, 2021 06:00

BrettRD marked this pull request as draft July 21, 2021 06:59

deduplicate remove functions

0c3c2db

Signed-off-by: BrettRD <[email protected]>

BrettRD marked this pull request as ready for review July 28, 2021 00:08

JanStaschulat approved these changes Jul 28, 2021

View reviewed changes

JanStaschulat merged commit 700ea24 into ros2:foxy Jul 28, 2021

mergify bot mentioned this pull request Jul 28, 2021

refactor #116 remove callback_type (backport #154) #163

Merged

mergify bot mentioned this pull request Jul 28, 2021

refactor #116 remove callback_type (backport #154) #164

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor #116 remove callback_type #154

refactor #116 remove callback_type #154

BrettRD commented Jul 21, 2021 •

edited

Loading

codecov-commenter commented Jul 21, 2021 •

edited

Loading

JanStaschulat Jul 28, 2021

JanStaschulat Jul 28, 2021

JanStaschulat commented Jul 28, 2021

mergify bot commented Jul 28, 2021

JanStaschulat commented Jul 28, 2021

mergify bot commented Jul 28, 2021

refactor #116 remove callback_type #154

refactor #116 remove callback_type #154

Conversation

BrettRD commented Jul 21, 2021 • edited Loading

codecov-commenter commented Jul 21, 2021 • edited Loading

Codecov Report

JanStaschulat Jul 28, 2021

Choose a reason for hiding this comment

JanStaschulat Jul 28, 2021

Choose a reason for hiding this comment

JanStaschulat commented Jul 28, 2021

mergify bot commented Jul 28, 2021

JanStaschulat commented Jul 28, 2021

mergify bot commented Jul 28, 2021

BrettRD commented Jul 21, 2021 •

edited

Loading

codecov-commenter commented Jul 21, 2021 •

edited

Loading